osxkeychain: define build targets in the top-level Makefile.#2046
Open
KojiNakamaru wants to merge 1 commit intogitgitgadget:masterfrom
Open
osxkeychain: define build targets in the top-level Makefile.#2046KojiNakamaru wants to merge 1 commit intogitgitgadget:masterfrom
KojiNakamaru wants to merge 1 commit intogitgitgadget:masterfrom
Conversation
Author
|
/preview |
|
Preview email sent as pull.2046.git.1770746103768.gitgitgadget@gmail.com |
Author
|
/submit |
|
Submitted as pull.2046.git.1770746461307.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Koji Nakamaru via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Makefile | 17 +++++++
> contrib/credential/osxkeychain/Makefile | 65 +++----------------------
> 2 files changed, 23 insertions(+), 59 deletions(-)
Nice simplification overall.
> diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
> index c68445b82d..ddb29f0563 100644
> --- a/contrib/credential/osxkeychain/Makefile
> +++ b/contrib/credential/osxkeychain/Makefile
> @@ -1,66 +1,13 @@
> # The default target of this Makefile is...
> all:: git-credential-osxkeychain
>
> -include ../../../config.mak.uname
> --include ../../../config.mak.autogen
> --include ../../../config.mak
> +git-credential-osxkeychain:
> + cd ../../..; make contrib/credential/osxkeychain/git-credential-osxkeychain
Let's not write "make", but stick to $(MAKE), perhaps like
$(MAKE) -C ../../.. contrib/credential/osxkeychain/$@
> +install:
> + cd ../../..; make install-git-credential-osxkeychain
>
> clean:
> - $(RM) git-credential-osxkeychain git-credential-osxkeychain.o
> + cd ../../..; make clean-git-credential-osxkeychain
Likewise. |
|
This patch series was integrated into seen via git@9d91fc6. |
|
This patch series was integrated into seen via git@1a1c338. |
The fix for git-credential-osxkeychain in 4580bcd (osxkeychain: avoid incorrectly skipping store operation) introduced linkage with libgit.a, and its Makefile was adjusted accordingly. However, the build fails as of 864f55e because several macOS-specific refinements were applied to the top-level Makefile and config.mak.uname, such as: - 363837a (macOS: make Homebrew use configurable, 2025-12-24) - cee341e (macOS: use iconv from Homebrew if needed and present, 2025-12-24) - d281241 (utf8.c: enable workaround for iconv under macOS 14/15, 2026-01-12) Since libgit.a and its corresponding header files depend on many flags defined in the top-level Makefile, these flags must be consistently defined when building git-credential-osxkeychain. Continuing to manually adjust the git-credential-osxkeychain Makefile is cumbersome and fragile. Define the build targets for git-credential-osxkeychain in the top-level Makefile and modify its local Makefile to simply rely on those targets. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Koji Nakamaru <koji.nakamaru@gree.net>
KojiNakamaru
force-pushed
the
fix/osxkeychain-makefile
branch
from
0aa80d1 to
8c5d75f
Compare
|
Koji Nakamaru wrote on the Git mailing list (how to reply to this email): On Wed, Feb 11, 2026 at 4:50 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Koji Nakamaru via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Makefile | 17 +++++++
> > contrib/credential/osxkeychain/Makefile | 65 +++----------------------
> > 2 files changed, 23 insertions(+), 59 deletions(-)
>
> Nice simplification overall.
>
> > diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
> > index c68445b82d..ddb29f0563 100644
> > --- a/contrib/credential/osxkeychain/Makefile
> > +++ b/contrib/credential/osxkeychain/Makefile
> > @@ -1,66 +1,13 @@
> > # The default target of this Makefile is...
> > all:: git-credential-osxkeychain
> >
> > -include ../../../config.mak.uname
> > --include ../../../config.mak.autogen
> > --include ../../../config.mak
> > +git-credential-osxkeychain:
> > + cd ../../..; make contrib/credential/osxkeychain/git-credential-osxkeychain
>
> Let's not write "make", but stick to $(MAKE), perhaps like
>
> $(MAKE) -C ../../.. contrib/credential/osxkeychain/$@
>
> > +install:
> > + cd ../../..; make install-git-credential-osxkeychain
> >
> > clean:
> > - $(RM) git-credential-osxkeychain git-credential-osxkeychain.o
> > + cd ../../..; make clean-git-credential-osxkeychain
>
> Likewise.
Thank you for the review. I'll apply these changes and submit the next
iteration soon.
--
Koji Nakamaru |
Author
|
/submit |
|
Submitted as pull.2046.v2.git.1770775169908.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Koji Nakamaru via GitGitGadget" <gitgitgadget@gmail.com> writes:
> -+ cd ../../..; make install-git-credential-osxkeychain
> ++ $(MAKE) -C ../../.. install-git-credential-osxkeychain
Will replace and mark for 'next'. Will squash the double spaces
after "-C" into one while doing so.
Thanks. |
|
Koji Nakamaru wrote on the Git mailing list (how to reply to this email): On Thu, Feb 12, 2026 at 12:37 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Koji Nakamaru via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > -+ cd ../../..; make install-git-credential-osxkeychain
> > ++ $(MAKE) -C ../../.. install-git-credential-osxkeychain
>
> Will replace and mark for 'next'. Will squash the double spaces
> after "-C" into one while doing so.
Thank you again for the squash and for marking it for 'next'.
--
Koji Nakamaru |
|
This branch is now known as |
|
This patch series was integrated into seen via git@ba5d4e8. |
|
This patch series was integrated into seen via git@06f3f5c. |
|
There was a status update in the "Cooking" section about the branch Simplify build procedure for oxskeychain (in contrib/). Will merge to 'next'? source: <pull.2046.v2.git.1770775169908.gitgitgadget@gmail.com> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes since v1:
cc: Junio C Hamano gitster@pobox.com